WIP: Add Azure VAP #321

racheljpg · 2024-10-17T13:23:55Z

Hello! This is a WIP to ask some questions on the generation for the Azure VAP to stop the deletion of Azure InfraClusters/IdentityReferences/whatever else we want to include. Thanks!

openshift-ci · 2024-10-17T13:23:59Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci · 2024-10-17T13:24:28Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign nrb for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

racheljpg

Hey @damdo, pinging you here as I have some questions about this and thought it was easier to ask here on a draft PR

racheljpg · 2024-10-17T13:25:36Z

openshift/infrastructure-components-openshift.yaml

+    apiVersion: admissionregistration.k8s.io/v1beta1
+    kind: ValidatingAdmissionPolicy
+    metadata:
+      name: openshift-cluster-api-protect-azureclusteridentities


I'm presuming this has to be it's own separate thing, and that it cannot be included in the VAP above by adding azureclusteridentities as a resource?

racheljpg · 2024-10-17T13:26:11Z

openshift/infrastructure-components-openshift.yaml

+        apiVersion: config.openshift.io/v1
+        kind: Infrastructure
+      validations:
+      - expression: '!(oldObject.metadata.name == params.status.infrastructureName)'


aware that this message isn't the most useful thing here - will update once I've tidied up a little

racheljpg · 2024-10-17T13:27:13Z

openshift/infrastructure-components-openshift.yaml

+        parameterNotFoundAction: Deny
+      policyName: openshift-cluster-api-protect-azureclusteridentities
+      validationActions:
+      - Deny


do we want to include anything else that we don't want to be deleted, or are the cluster and the identity reference enough?
(ignore the lack of a new line, will fix:) )

damdo · 2024-10-22T12:18:16Z

/test ?

openshift-ci · 2024-10-22T12:18:20Z

@damdo: The following commands are available to trigger required jobs:

/test e2e-azure
/test e2e-azure-capi-techpreview
/test e2e-azure-serial
/test images
/test security
/test unit

The following commands are available to trigger optional jobs:

/test e2e-azure-manual-oidc
/test e2e-azure-techpreview
/test okd-scos-images
/test verify-commits

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-cluster-api-provider-azure-master-e2e-azure
pull-ci-openshift-cluster-api-provider-azure-master-e2e-azure-capi-techpreview
pull-ci-openshift-cluster-api-provider-azure-master-e2e-azure-serial
pull-ci-openshift-cluster-api-provider-azure-master-e2e-azure-techpreview
pull-ci-openshift-cluster-api-provider-azure-master-images
pull-ci-openshift-cluster-api-provider-azure-master-security
pull-ci-openshift-cluster-api-provider-azure-master-unit
pull-ci-openshift-cluster-api-provider-azure-master-verify-commits

In response to this:

/test ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-ci · 2024-10-22T12:18:39Z

@damdo: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test e2e-azure
/test e2e-azure-capi-techpreview
/test e2e-azure-serial
/test images
/test security
/test unit

The following commands are available to trigger optional jobs:

/test e2e-azure-manual-oidc
/test e2e-azure-techpreview
/test okd-scos-images
/test verify-commits

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-cluster-api-provider-azure-master-e2e-azure
pull-ci-openshift-cluster-api-provider-azure-master-e2e-azure-capi-techpreview
pull-ci-openshift-cluster-api-provider-azure-master-e2e-azure-serial
pull-ci-openshift-cluster-api-provider-azure-master-e2e-azure-techpreview
pull-ci-openshift-cluster-api-provider-azure-master-images
pull-ci-openshift-cluster-api-provider-azure-master-security
pull-ci-openshift-cluster-api-provider-azure-master-unit
pull-ci-openshift-cluster-api-provider-azure-master-verify-commits

In response to this:

/test pull-ci-openshift-cluster-api-provider-azure-master-e2e-azure-capi-techpreview

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

damdo · 2024-10-22T12:23:53Z

/retitle WIP: Add Azure VAP

damdo · 2024-10-22T12:24:36Z

/test e2e-azure-capi-techpreview

damdo · 2024-10-23T13:18:21Z

openshift/infrastructure-components-openshift.yaml

+      matchConstraints:
+        resourceRules:
+        - apiGroups:
+          - infrastructure.cluster.x-k8s.io


This will need to match the apiGroup of a Secret, which is the core one.

Thanks, updated!

damdo · 2024-10-25T12:41:34Z

openshift/infrastructure-components-openshift.yaml

+        message: InfraCluster resources with metadata.name corresponding to the cluster
+          infrastructureName cannot be deleted.
+---
+    apiVersion: admissionregistration.k8s.io/v1beta1


Let's change all of these to v1, and make sure the syntax/fields are ok with v1

damdo · 2024-10-25T12:41:46Z

openshift/infrastructure-components-openshift.yaml

+      validationActions:
+      - Deny
+---
+    apiVersion: admissionregistration.k8s.io/v1beta1


damdo · 2024-10-25T12:41:54Z

openshift/manifests/0000_30_cluster-api_04_cm.infrastructure-azure.yaml

@@ -11025,6 +11025,129 @@ data:
        targetPort: webhook-server
      selector:
        cluster.x-k8s.io/provider: infrastructure-azure
+    ---
+    apiVersion: admissionregistration.k8s.io/v1beta1


damdo · 2024-10-25T12:41:59Z

openshift/manifests/0000_30_cluster-api_04_cm.infrastructure-azure.yaml

+        message: InfraCluster resources with metadata.name corresponding to the cluster
+          infrastructureName cannot be deleted.
+    ---
+    apiVersion: admissionregistration.k8s.io/v1beta1


damdo · 2024-10-25T13:00:14Z

openshift/infrastructure-components-openshift.yaml

@@ -11023,3 +11023,126 @@ spec:
    targetPort: webhook-server
  selector:
    cluster.x-k8s.io/provider: infrastructure-azure
+---
+    apiVersion: admissionregistration.k8s.io/v1


Indentation is wrong on these YAMLs (only on this file openshift/infrastructure-components-openshift.yaml), they should be starting at the beginning of the line

damdo · 2024-10-25T13:02:59Z

openshift/manifests/0000_30_cluster-api_04_cm.infrastructure-azure.yaml

+      ---
+      apiVersion: admissionregistration.k8s.io/v1
+      kind: ValidatingAdmissionPolicyBinding
+      metadata:
+        name: openshift-cluster-api-protect-azureclustersecrets
+      spec:
+        matchResources:
+          namespaceSelector:
+            matchLabels:
+              kubernetes.io/metadata.name: openshift-cluster-api
+        paramRef:
+          name: cluster
+          parameterNotFoundAction: Deny
+        policyName: openshift-cluster-api-protect-azureclustersecrets
+        validationActions:
+        - Deny


The indentation of this file is off (too much on the right end side, which is throwing off the parser with:
E1025 10:54:25.739990 1 controller.go:316] "Reconciler error" err="error during reconcile: error applying CAPI provider \"azure\" components: error getting provider components: error parsing provider component at position 32 to unstructured: error while decoding YAML to runtime object: error while decoding YAML object: yaml: line 29: could not find expected ':'" controller="CapiInstallerController" controllerGroup="config.openshift.io" controllerKind="ClusterOperator" ClusterOperator="cluster-api" namespace="" name="cluster-api" reconcileID="ff9c7798-9de6-4629-a95e-c358aaecf1f2" E1025 10:54:29.500601 1 kind.go:71] "if kind is a CRD, it should be installed before calling Start" err="failed to get restmapping: no matches for kind \"AzureCluster\" in version \"infrastructure.cluster.x-k8s.io/v1beta1\"" logger="controller-runtime.source.EventHandler" kind="AzureCluster.infrastructure.cluster.x-k8s.io"

nrb · 2024-11-04T20:49:16Z

#322 now includes the VAPs; I couldn't get around generating the manifests without the VAPs once I updated manifests-gen, it would cause a verification job to fail because the committed version and the generated version were different.

Along those lines, I wonder if this particular PR should be done as a Kustomization patch, since infrastructure-components{,-openshift}.yaml, plus the resulting ConfigMap, are generated by manifests-gen.

openshift-merge-robot · 2025-01-16T11:40:13Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

damdo · 2025-01-16T12:42:33Z

@racheljpg are you still planning to finish this off?

racheljpg · 2025-01-16T13:00:16Z

@damdo yes, I think we still need this! Will pick it back up ASAP

openshift-bot · 2025-04-17T01:01:05Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

damdo · 2025-04-17T09:00:00Z

@racheljpg when we do this, we'll need to also need to update the other providers to use v1 instead of v1beta1

racheljpg · 2025-04-17T12:35:58Z

/remove-lifecycle stale

openshift-ci · 2025-07-01T15:46:45Z

@racheljpg: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/hypershift-e2e-aks	`050cca8`	link	true	`/test hypershift-e2e-aks`
ci/prow/e2e-azure-serial-1of2	`050cca8`	link	true	`/test e2e-azure-serial-1of2`
ci/prow/e2e-azure-serial-2of2	`050cca8`	link	true	`/test e2e-azure-serial-2of2`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

damdo · 2025-07-17T14:22:26Z

@racheljpg We should likely move these to cluster-capi-operator

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 17, 2024

racheljpg force-pushed the azureVAP branch from 4c273c3 to 742917e Compare October 17, 2024 13:24

racheljpg commented Oct 17, 2024

View reviewed changes

openshift-ci bot changed the title ~~Add Azure VAP~~ WIP: Add Azure VAP Oct 22, 2024

racheljpg force-pushed the azureVAP branch 2 times, most recently from c431373 to 37e035f Compare October 23, 2024 13:16

damdo reviewed Oct 23, 2024

View reviewed changes

racheljpg force-pushed the azureVAP branch 2 times, most recently from f5e6f18 to 13aa469 Compare October 24, 2024 13:30

damdo reviewed Oct 25, 2024

View reviewed changes

racheljpg force-pushed the azureVAP branch from 13aa469 to 4efb79a Compare October 25, 2024 12:59

damdo reviewed Oct 25, 2024

View reviewed changes

Add VAP for azurecluster

050cca8

racheljpg force-pushed the azureVAP branch from 4efb79a to 050cca8 Compare October 25, 2024 13:28

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 16, 2025

openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 17, 2025

openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 17, 2025

WIP: Add Azure VAP #321

Are you sure you want to change the base?

WIP: Add Azure VAP #321

Uh oh!

Conversation

racheljpg commented Oct 17, 2024

Uh oh!

openshift-ci bot commented Oct 17, 2024

Uh oh!

openshift-ci bot commented Oct 17, 2024

Uh oh!

racheljpg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

damdo commented Oct 22, 2024

Uh oh!

openshift-ci bot commented Oct 22, 2024

Uh oh!

openshift-ci bot commented Oct 22, 2024

Uh oh!

damdo commented Oct 22, 2024

Uh oh!

damdo commented Oct 22, 2024

Uh oh!

damdo Oct 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nrb commented Nov 4, 2024

Uh oh!

openshift-merge-robot commented Jan 16, 2025

Uh oh!

damdo commented Jan 16, 2025

Uh oh!

racheljpg commented Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-bot commented Apr 17, 2025

Uh oh!

damdo commented Apr 17, 2025

Uh oh!

racheljpg commented Apr 17, 2025

Uh oh!

openshift-ci bot commented Jul 1, 2025

Uh oh!

damdo commented Jul 17, 2025

Uh oh!

Uh oh!

damdo Oct 23, 2024 •

edited

Loading

racheljpg commented Jan 16, 2025 •

edited

Loading